Require Bitbucket DC bot token + username when BBDC auth enabled by ak684 · Pull Request #732 · OpenHands/OpenHands-Cloud

ak684 · 2026-06-18T02:28:52Z

What

When Bitbucket Data Center authentication is enabled, require the bot account:

bitbucket_data_center_bot_token → now required: true (was "strongly recommended").
New bitbucket_data_center_bot_username (the bot account's username/slug) → required: true, wired to the BITBUCKET_DATA_CENTER_BOT_USERNAME env (_env.yaml / openhands.yaml / values.yaml).
openhands chart 0.7.61 → 0.7.62.

Why

OpenHands must post BBDC comments/reactions as the bot, never as the mentioning user — a user-authored reply containing @openhands re-triggers the webhook and spawns a duplicate conversation (confirmed from a customer support bundle). The app-side fix (OpenHands enterprise) posts as the bot and ignores the bot's own comments using the configured username (slug) — BBDC payloads carry the slug, not a reliable email. Making both required ensures the integration is always in the correct, non-duplicating configuration. Pairs with the OpenHands enterprise PR (BBDC → Jira DC parity).

all-hands-bot · 2026-06-18T02:40:54Z

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

all-hands-bot

🟡 Acceptable - The Replicated-side wiring is simple, but direct Helm installs still don't fail fast for the newly required Bitbucket Data Center bot identity.

[IMPROVEMENT OPPORTUNITIES]

[charts/openhands/templates/_env.yaml, Line 204] Configuration Contract: The chart renders an empty username and still permits the bot token secret key to be absent, so Helm consumers can enable BBDC without the bot identity this PR says is required.

[RISK ASSESSMENT]

[Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
This changes authentication/integration configuration for Bitbucket Data Center. Replicated installs get stricter validation, but the direct Helm path can remain silently misconfigured unless the new required values are enforced and documented.

VERDICT:
❌ Needs follow-up: Please align the Helm chart path (fail fast for missing bitbucketDataCenter.botUsername/bot token and update the README secret example) or clarify that this requirement is Replicated-only.

KEY INSIGHT:
The data flow is correct for Replicated, but the chart's public contract still allows the invalid configuration the PR is trying to eliminate.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.

Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.

When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

all-hands-bot · 2026-06-18T02:44:18Z

 - name: BITBUCKET_DATA_CENTER_HOST
  value: {{ .Values.bitbucketDataCenter.host }}
+- name: BITBUCKET_DATA_CENTER_BOT_USERNAME
+  value: {{ .Values.bitbucketDataCenter.botUsername | quote }}


🟠 Important: This adds the env var, but direct Helm installs render it as an empty string when bitbucketDataCenter.enabled=true and bitbucketDataCenter.botUsername is unset. In the same chart path, BITBUCKET_DATA_CENTER_BOT_TOKEN is still sourced from an optional secret key, so the chart still permits exactly the bot-less BBDC configuration this PR is trying to forbid. Please fail fast here (for example, required "bitbucketDataCenter.botUsername is required when bitbucketDataCenter.enabled=true" .Values.bitbucketDataCenter.botUsername and make the bot-token secret key non-optional or add an explicit required value/schema path), and update charts/openhands/README.md's BBDC secret/values example to include both bot credentials.

Integration credentials (bot tokens/usernames, service-account emails/PATs) are enforced as required in replicated/config.yaml, not via Helm `required` in the chart -- consistent across all integrations (Jira DC, Bitbucket DC, ...). Records the convention so reviews don't flag KOTS-enforced-not-Helm-enforced wiring as a gap; tightening the bare-helm path is a deliberate chart-wide follow-up, out of scope for an integration-wiring PR.

ak684 · 2026-06-18T03:26:20Z

Thanks for the careful read. I want to push back on the one improvement item, with parity evidence, rather than change it here — and I've documented the convention so it's clear going forward.

The bot identity is enforced — at the KOTS layer, exactly like every other integration

We do support bare-Kubernetes/Helm customers, so this isn't "raw Helm is unsupported." The point is that required integration credentials are enforced in replicated/config.yaml, which is the install path customers actually use:

bitbucket_data_center_bot_token → required: true + when: bitbucket_data_center_auth_enabled (config.yaml L565)
bitbucket_data_center_bot_username → required: true + when: … (L573)

This is identical to how Jira DC already ships:

jira_data_center_service_account_email → required: true + when: jira_data_center_enabled (L643)
jira_data_center_service_account_pat → required: true (L652)

And in the chart, no integration uses Helm required to fail-fast — JDC's email/PAT are secretKeyRef … optional: true (_env.yaml, secret jira-dc-app), same as BBDC's bot token (bitbucket-data-center-app/bot-token, optional: true). BBDC's botUsername is a plain value because a username slug isn't a secret. So this PR follows the established pattern exactly; it doesn't introduce a new gap relative to JDC.

The bare-`helm install` fail-fast is a real idea — but a deliberate, chart-wide one

Tightening the chart so a direct helm install --set …enabled=true aborts when the bot/service-account identity is missing is worth doing — but it should be done consistently for all integrations (Jira DC, Bitbucket DC, Azure DevOps, …) in its own focused PR, not bolted onto BBDC alone (which would make BBDC stricter than its own reference integration). Most of those identities are secret-sourced, so a clean Helm required needs a small, deliberate design rather than a one-off.

I've recorded this convention in .agents/skills/code-review.md (Integration Credential Enforcement) so future integration-wiring PRs aren't flagged for following it.

Re-requesting review.

all-hands-bot · 2026-06-18T03:27:13Z

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

all-hands-bot

🟢 Good taste — Minimal, focused change that correctly wires a new required BBDC field through the full config-to-chart stack.

The previous review flagged that direct Helm installs don't fail-fast for the missing bot identity. This PR explicitly addresses that concern by updating .agents/skills/code-review.md with the architectural rationale: KOTS is the primary install path for every customer, and by established convention credentials are enforced at the KOTS layer (replicated/config.yaml with required: true) rather than via Helm's required function. The pattern is consistent across all integrations (Jira DC, Azure DevOps, etc.) — secrets use secretKeyRef … optional: true in the chart, non-secret values default to empty. The documentation update is a legitimate and accurate description of the system's deployment architecture.

The change itself is clean:

replicated/config.yaml: bot token upgraded from "strongly recommended" to required: true; new bitbucket_data_center_bot_username field added with required: true and correct type: text (not password).
replicated/openhands.yaml: new config option wired to bitbucketDataCenter.botUsername.
_env.yaml: BITBUCKET_DATA_CENTER_BOT_USERNAME emitted under the existing bitbucketDataCenter.enabled guard, using | quote (consistent with — and actually safer than — how BITBUCKET_DATA_CENTER_HOST is rendered without the filter).
values.yaml: botUsername: "" default follows the same pattern as host: "".
Chart version bump is appropriate.

No breaking changes, no new dependencies, no security concerns. The fix addresses a real, confirmed customer problem (duplicate job triggers from bot's own comment replies).

[RISK ASSESSMENT]

[Overall PR] ⚠️ Risk Assessment: 🟢 LOW
Config/infrastructure changes only. Making credentials required is a hardening change. All wiring follows the established pattern. The KOTS-layer enforcement is the correct mechanism for this deployment model.

VERDICT:
✅ Worth merging — Correct fix, clean implementation, well-documented architectural rationale.

KEY INSIGHT:
Enforcing required bot credentials at the KOTS config layer (not the Helm chart) is the right pattern for this deployment model — the .agents/skills update documents this convention so future reviewers don't re-raise the same concern.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

- Drop redundant "(required)" from the bot token title; KOTS already marks required fields and the Jira DC fields use plain titles. - Reword the help text now the token is required: we post as the bot so its replies don't re-trigger, and instead of "do not sign in" give the real reason -- OpenHands ignores comments this bot authors, so use a dedicated account. - Revert the .agents/skills/code-review.md convention note now that the review has approved.

Drop the dedicated-account caveat from the help text; it will live in docs/ where there is room to explain it.

Require Bitbucket DC bot token + bot username when BBDC auth enabled

3c75dc2

ak684 mentioned this pull request Jun 18, 2026

fix: post Bitbucket DC comments as the bot account and background webhook handling OpenHands/OpenHands#14881

Merged

1 task

ak684 requested a review from all-hands-bot June 18, 2026 02:39

all-hands-bot reviewed Jun 18, 2026

View reviewed changes

ak684 requested a review from all-hands-bot June 18, 2026 03:26

all-hands-bot approved these changes Jun 18, 2026

View reviewed changes

ak684 added 2 commits June 17, 2026 23:37

chore(config): trim BBDC bot token help text

56e5daa

Drop the dedicated-account caveat from the help text; it will live in docs/ where there is room to explain it.

ak684 merged commit a0dac7b into main Jun 18, 2026
28 checks passed

ak684 deleted the alona/bbdc-bot-token-required branch June 18, 2026 03:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Require Bitbucket DC bot token + username when BBDC auth enabled#732

Require Bitbucket DC bot token + username when BBDC auth enabled#732
ak684 merged 4 commits into
mainfrom
alona/bbdc-bot-token-required

ak684 commented Jun 18, 2026 •

edited

Loading

Uh oh!

all-hands-bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

all-hands-bot left a comment

Uh oh!

all-hands-bot Jun 18, 2026

Uh oh!

ak684 commented Jun 18, 2026

Uh oh!

all-hands-bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

all-hands-bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ak684 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Uh oh!

all-hands-bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

all-hands-bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

ak684 commented Jun 18, 2026

The bot identity is enforced — at the KOTS layer, exactly like every other integration

The bare-helm install fail-fast is a real idea — but a deliberate, chart-wide one

Uh oh!

all-hands-bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ak684 commented Jun 18, 2026 •

edited

Loading

all-hands-bot commented Jun 18, 2026 •

edited

Loading

The bare-`helm install` fail-fast is a real idea — but a deliberate, chart-wide one

all-hands-bot commented Jun 18, 2026 •

edited

Loading